Bind only $x, not x in def f($x): ...#3520
Conversation
01mf02
changed the title
$x, not x in def f($x): ....$x, not x in def f($x): ...
|
By the way, a little note on LLM usage: I tested whether LLMs could be useful to do this fix. I gave ChatGPT the following prompts:
It gave me a completely unusable patch, then told me: "If you want, I can refine this into a PR-ready patch against a specific jq version (with exact line numbers and tests).". I replied:
Behold its result, headlined boldly by "✅ Patch (ready to submit)": diff --git a/src/compile.c b/src/compile.c
index 7c1e5ab..e4d9abc 100644
--- a/src/compile.c
+++ b/src/compile.c
@@ -1308,18 +1308,35 @@ static int compile_funcdef(block* b, struct locfile* lf, jv name, jv args, block
int nformals = jv_array_length(jv_copy(args));
for (int i = 0; i < nformals; i++) {
- jv arg = jv_array_get(jv_copy(args), i);
- const char* argname = jv_string_value(arg);
-
- // Existing behavior: bind both positional and variable
- block_bind_argument(b, argname);
- block_bind_variable(b, argname);
+ jv arg = jv_array_get(jv_copy(args), i);
+ const char* raw = jv_string_value(arg);
+
+ if (raw[0] == '$') {
+ // Variable argument: strip leading '$'
+ const char* varname = raw + 1;
+
+ if (varname[0] == '\0') {
+ locfile_locate(lf, jv_copy(arg));
+ FAIL(*b, "invalid variable name \"$\"");
+ }
+
+ // Bind ONLY as variable
+ block_bind_variable(b, varname);
+
+ // NOTE: intentionally NOT calling block_bind_argument()
+ } else {
+ // Positional argument: preserve existing semantics
+ block_bind_argument(b, raw);
+ block_bind_variable(b, raw);
+ }
}
jv_free(args);
// Compile function body
block body_block = gen_noop();
int r = compile(b, lf, body, &body_block);
if (r != 0)
return r;Hallucination at its finest. The file exists, but neither does the function ... However, it brought me on track with the In this case, however, it would have been fastest to just look up the commit that added the "regular argument" functionality (32c08d4). Which is what I ended up doing anyway. |
1 participant
@wader brought to my attention in 01mf02/jaq#428 that jq currently has the following behaviour:
That means that in a function
f($a), bothaand$aare bound. This has already been pointed out by @pkoppstein in @nicowilliams's PR #524, which added "regular" definition arguments (where "regular" means "$..." here).This PR tries to rectify this, by changing how to compile regular definition arguments. Let us use @wader's example above and see what it rewrites to:
The only change is that it binds any regular argument as
@regular, which is a variable name that cannot be referenced by the user.This excludes a few programs that previously compiled (including @wader's example above):
Furthermore, it changes the meaning of some programs: